[#55581] popover for user information on hover #17255
Conversation
EinLama
force-pushed
the
feature/55581-popover-for-user-information-on-hover
branch
from
1f5454c
to
4653b53
Compare
There was a problem hiding this comment.
Hi @EinLama
as discussed, the amount of cards feels rather annoying. I'd like the challenge the requirement that the card should really be shown all the time. Alternatively, the delay must be larger. But that has to be discussed with the product team.
Until then, here are the first findings from clicking through:
- Hovering a placeholder user leads to an error
- As a non-admin user, there is an error as well due to a missing permission
- Opening the share modal on a WP creates a black overlay
- When the LogTime modal on a WP is opened, and the user dropdown is opened: Hovering the avatar, will close the logTime modal
- The emailAdress is supposed to smaller (12px)
- When having a very long name, it overflows the card without truncation or wrapping:
- When the hovercard is opened at the very bottom of the page, it closes immideatly
Bildschirmaufnahme.2024-11-22.um.14.55.49.mov
For the record, there are some places where the card is not shown yet. But as said above, I'd wait until the decision of the product team
- Team planner module
- Admin/user table
- Project overview / project attribute of type user
- Project activity page
- Users page (
/users/:id) - My Page / News widget
- Multi-User custom field
EinLama
force-pushed
the
feature/55581-popover-for-user-information-on-hover
branch
from
f3869f7
to
95183a7
Compare
| # or | ||
| # "Member of group1, group2 and 3 more" | ||
| # The latter string is cut off since the complete list of group names would exceed the allowed `max_length`. | ||
| def group_membership_summary(max_length = 40) |
There was a problem hiding this comment.
Where is the value 40 coming from?
There was a problem hiding this comment.
A true magic number 🔮🧙🏻♂️ found by, uh... guessing and trying it out! 🪄
Seemed sensible when used with the English translation and should look okay with most other languages. The caveat is that only the group names are considered when deciding where to shorten the list. To make it perfect, we'd have to load the translation first, factor it's length into the calculation and THEN decide where to cut off the group list. I figured the value 40 is good enough in most cases and strikes a balance between pleasant looks and code complexity :)
| @@ -48,7 +48,7 @@ export class UserDisplayField extends DisplayField { | |||
| element, | |||
| this.attribute, | |||
| { hide: false, link: false }, | |||
| { hide: false, size: 'medium' }, | |||
| { hide: false, size: 'medium', hoverCard: { url: this.attribute.url } }, | |||
There was a problem hiding this comment.
ESLint is somehow complaining correctly here.this.attribute can be anything, so we do not know for sure whether a url is defined. Ideally this case would at least be catched before.
There was a problem hiding this comment.
There were other issues with this class, unrelated to this PR. I have made an attempt at fixing them. The compiler is happy, so far is the runtime. Will observe if this causes problems somewhere in the application.
frontend/src/app/core/setup/globals/global-listeners/hover-card-trigger.service.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/shared/components/principal/principal-renderer.service.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/shared/components/principal/principal-renderer.service.ts
Outdated
Show resolved
Hide resolved
EinLama
force-pushed
the
feature/55581-popover-for-user-information-on-hover
branch
from
1d9c209
to
0b824d4
Compare
EinLama
force-pushed
the
feature/55581-popover-for-user-information-on-hover
branch
2 times, most recently
from
d1ef552
to
eeaa6cf
Compare
EinLama
force-pushed
the
feature/55581-popover-for-user-information-on-hover
branch
from
709ca64
to
eba7ab8
Compare
This makes the hover card feel less annoying when letting the mouse pointer wander across a page.
We will treat all hover cards the same to reduce the complexity.
By default, it will now be shown. Since we want to have it almost anywhere on the page, it makes sense that `true` is the default setting.
It's hover cards all the way down.
To align with the user profile. We don't want to show hidden groups here and expose secret information.
They conflict with each other, so we disable them.
This happened when the hover card rendered on top (e.g. when you scrolled down a bit and there was no space below the trigger). The hover card will now also consider the trigger when deciding whether the mouse left the card or not.
There is only one possible slot to inject a modal. If that slot is taken, abort!
EinLama
force-pushed
the
feature/55581-popover-for-user-information-on-hover
branch
from
6b862e2
to
7679081
Compare
Ticket
https://community.openproject.org/wp/55581
What are you trying to accomplish?
User hover cards when hovering over the avatar. I tried also showing the hover card when hovering over the name, but it felt wonky and overly verbose. EDIT: as discussed in the frontend daily, the wonkyness is no longer present with an increased delay before showing a hover card. So showing it for names is an option now. We decided that I will create an open point for this topic so that we can decide how to proceed with this.
Screenshots
What approach did you choose and why?
Merge checklist